Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.48%. Comparing base (64c246b) to head (797a23c).
⚠️ Report is 2 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1703      +/-   ##
==========================================
- Coverage   92.49%   92.48%   -0.01%     
==========================================
  Files         157      157              
  Lines       10489    10489              
==========================================
- Hits         9702     9701       -1     
- Misses        787      788       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Would this ever cause any problem? I think it is no issue in the current implementation, but it could be a hidden bug if the implementation changes. Not sure if that is good enough justification for the change :-D

_identify_inactive_browsers is sync, so it will run through the iteration without anyone modifying its list.
_close_inactive_browsers is async, so it can yield in the middle of the iteration and the _identify_inactive_browsers can start changing the list. But the worst-case scenario is that it will miss some freshly added inactive_browsers, which does not matter at all, because it is a RecurringTask that runs periodically, so they will be closed in the next run anyway.

The point is, when you remove an item from a list during iteration, the iterator's index doesn't adjust, and the next item will be skipped.

lst = [1, 2, 3, 4]
for x in lst:
    print(x)
    if x == 2:
        lst.remove(x)
print(lst)

$ python run.py 
1
2
4
[1, 3, 4]

See e.g. this https://stackoverflow.com/questions/1637807/modifying-list-while-iterating

So this is clearly a bug.

The point is, when you remove an item from a list during iteration, the iterator's index doesn't adjust, and the next item will be skipped.
...

Sure, but that is not the case in our code. The case in our code is that an item can be added to the list during the iteration, so such an item will be missed in the current loop, but since this function runs periodically, it does not matter, as it will be handled next time the function runs.